sql: implement CREATE SCHEMA ... AUTHORIZATION
#53583
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 6 of 6 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @lucy-zhang, @rohany, and @solongordon)
pkg/sql/create_schema.go, line 73 at r1 (raw file):
Quoted 4 lines of code…
// Check validity of the schema name. if err := schemadesc.IsSchemaNameValid(schemaName); err != nil { return err }
Should we do this check before the existence check?
pkg/sql/parser/sql.y, line 5297 at r1 (raw file):
// %Category: DDL // %Text: // CREATE SCHEMA [IF NOT EXISTS] <schemaname> [AUTHORIZATION <rolename>]
nit: should this be CREATE SCHEMA [IF NOT EXISTS] { <schemaname> | [ [<schemaname>] AUTHORIZATION <rolename>] }
|
Done, addressed both comments bors r+ |
|
Build failed (retrying...): |
There was a problem hiding this comment.
Reviewable status:
pkg/sql/create_schema.go, line 59 at r2 (raw file):
// Check validity of the schema name. if err := schemadesc.IsSchemaNameValid(schemaName); err != nil {
seems like re-arranging this broke some tests 😬
There was a problem hiding this comment.
Reviewable status:
pkg/sql/parser/sql.y, line 5297 at r2 (raw file):
// %Category: DDL // %Text: // CREATE SCHEMA [IF NOT EXISTS] { <schemaname> | [<schemaname>] [AUTHORIZATION <rolename>] }
also, I don't think this means the same thing as what I wrote. This seems to imply that you could omit both elements of the RHS. I think you could do:
CREATE SCHEMA [IF NOT EXISTS] { <schemaname> | [<schemaname>] AUTHORIZATION <rolename> }
|
Ah yeah you're right, can't have brackets around the |
|
this is failing ci - removing from the bors queue bors r- |
|
Canceled. |
Fixes cockroachdb#53559. This commit adds the `CREATE SCHEMA ... AUTHORIZATION` command. When authorization is provided, the target user is given ownership of the schema. If the schema name is not provided, then the schema is named the same name as the target role. Release justification: low risk updates to new functionality Release note (sql change): Support the `CREATE SCHEMA ... AUTHORIZATION` command.
|
Updated the help message. Seems like we need to keep the name check after the existence check. |
|
bors r+ |
|
bors r- |
|
Canceled. |
|
@ajwerner can you stamp this? |
There was a problem hiding this comment.
Reviewed 4 of 6 files at r1, 2 of 2 files at r3.
Reviewable status:
There was a problem hiding this comment.
Reviewable status:
|
bors r+ |
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Build succeeded: |
Fixes #53559.
This commit adds the
CREATE SCHEMA ... AUTHORIZATIONcommand. Whenauthorization is provided, the target user is given ownership of the
schema. If the schema name is not provided, then the schema is named the
same name as the target role.
Release justification: low risk updates to new functionality
Release note (sql change): Support the
CREATE SCHEMA ... AUTHORIZATIONcommand.